Pull request overview

This PR migrates Flash’s project scanning and resource discovery from AST-based parsing to runtime importing + object inspection, enabling detection of dynamically constructed endpoints/resources while also fixing missing idleTimeout propagation.

Changes:

• Replace RemoteDecoratorScanner/ResourceDiscovery patterns with RuntimeScanner-driven import+inspect scanning across build/run flows.
• Update Endpoint to support nameless QB decorator mode (derive name from decorated target) while requiring name/id only for image= client mode and enforcing name for LB route registration.
• Propagate idleTimeout through manifest generation and runtime provisioning; update/trim tests accordingly.

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

1. Architecture shift — sound motivation, correct implementation

AST parsing breaking on computed decorators, conditional definitions, and programmatic resource construction is a real limitation. The __remote_config__ stamp pattern is the right fix: @remote and Endpoint both attach a dict at decoration time, and the scanner just walks live module members looking for it. Dynamic patterns that defeated AST parsing work correctly under import inspection.

The idleTimeout fix (manifest.py + resource_provisioner.py) and _wrapped_class on RemoteClassWrapper are clean bundled fixes that correctly close gaps in the manifest pipeline and class introspection.

2. Bug: partial build failure silently produces incomplete manifest

# build.py
if scanner.import_errors:
    console.print("\n[red bold]Failed to load:[/red bold]")
    for filename, err in scanner.import_errors.items():
        console.print(f"  [red]{filename}[/red]: {err}")
    if not remote_functions:      # ← only aborts if ALL modules failed
        raise typer.Exit(1)

If a user has five worker files and two fail to import, the build continues with a three-worker manifest, prints the errors in red, exits 0, and deploys. The user has a silently incomplete app.

The if not remote_functions guard is too permissive. At minimum, import failures should print a warning that the build is incomplete and require --allow-partial or similar to proceed. A hard stop on any import error is the safer default.

3. Issue: no timeout on module imports

_import_module_from_file() calls spec.loader.exec_module(module) with no timeout. Module-level blocking code — a database connection, time.sleep(), or an infinite loop — will hang flash build indefinitely with no output. The AST scanner was immune to this; runtime import is not. A per-file timeout (e.g., concurrent.futures with a 10s limit) would make failure explicit.

4. High: ~2,346 lines of scanner tests deleted, zero unit tests added

Three test files deleted:

• test_scanner.py — 1,480 lines
• test_scanner_endpoint.py — 600 lines
• test_scanner_load_balancer.py — 266 lines

The PR adds no new unit tests for RuntimeScanner. test_build_pipeline.py has 15 line-for-line updates but no new scenarios. What's now untested:

• _import_module_from_file() error paths (syntax error, missing dep, import exception)
• _find_remote_decorated() / _find_endpoint_instances() introspection edge cases
• _analyze_cross_calls_ast() with files that failed to import
• Cross-call detection partial-failure degradation (item 2 above)
• Module cleanup in finally block
• __flash_local__ flag propagation through _metadata_from_remote_config()
• LB route stamp correctness (method / path from @ep.get(...))

The old tests covered these exhaustively. The new implementation is simpler, but no behavioral guarantees are locked in.

5. Testing — what remains

Integration tests in test_build_pipeline.py and test_lb_remote_execution.py are updated and cover the happy path. The test name change ("test-gpu" → "test-gpu-fb") correctly reflects that runtime scanning reads the actual resource name (with suffix) rather than the AST string literal — that's a latent correctness improvement.

Verdict

The architectural direction is right and the implementation looks correct for the cases it targets. Two things should be addressed before merge: the partial-build silent-success behavior (item 2 — real user impact) and the unit test gap (item 4 — behavioral guarantees are unverified for error paths). The import timeout (item 3) is lower priority but worth a follow-up.